popupMenu: Hide separators when they aren't separating

A separator only makes sense if there are items on both
sides of it. There is quite a lot of code written
throughout the shell that manages the process of showing
and hiding separators as the items around those separators
change.

This commit drops all that code in favor of changes to the menu
implementation to dynamically hide or show separators as
appropriate, so the callers don't have to deal with it.

https://bugzilla.gnome.org/show_bug.cgi?id=657082

diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js
index 9ee304dab0c8262cb5f05f5661c0836159fd6a8f..228fc76e2858000aa0e520b6b041064080448e0f 100644 (file)
--- a/js/ui/popupMenu.js
+++ b/js/ui/popupMenu.js
@@ -860,6 +860,39 @@ PopupMenuBase.prototype = {
         }));
     },
 
+    _updateSeparatorVisibility: function(menuItem) {
+        let children = this.box.get_children();
+
+        let index = children.indexOf(menuItem.actor);
+
+        if (index < 0)
+            return;
+
+        let childBeforeIndex = index - 1;
+
+        while (childBeforeIndex >= 0 && !children[childBeforeIndex].visible)
+            childBeforeIndex--;
+
+        if (childBeforeIndex < 0
+            || children[childBeforeIndex]._delegate instanceof PopupSeparatorMenuItem) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        let childAfterIndex = index + 1;
+
+        while (childAfterIndex < children.length && !children[childAfterIndex].visible)
+            childAfterIndex++;
+
+        if (childAfterIndex >= children.length
+            || children[childAfterIndex]._delegate instanceof PopupSeparatorMenuItem) {
+            menuItem.actor.hide();
+            return;
+        }
+
+        menuItem.actor.show();
+    },
+
     addMenuItem: function(menuItem, position) {
         let before_item = null;
         if (position == undefined) {
@@ -891,6 +924,14 @@ PopupMenuBase.prototype = {
                 if (!open)
                     menuItem.menu.close(false);
             });
+        } else if (menuItem instanceof PopupSeparatorMenuItem) {
+            this._connectItemSignals(menuItem);
+
+            // updateSeparatorVisibility needs to get called any time the
+            // separator's adjacent siblings change visibility or position.
+            // open-state-changed isn't exactly that, but doing it in more
+            // precise ways would require a lot more bookkeeping.
+            this.connect('open-state-changed', Lang.bind(this, function() { this._updateSeparatorVisibility(menuItem); }));
         } else if (menuItem instanceof PopupBaseMenuItem)
             this._connectItemSignals(menuItem);
         else

diff --git a/js/ui/status/bluetooth.js b/js/ui/status/bluetooth.js
index d5069963f40f30706936bbe6c8c9ec54ba7a4c9e..2970b2c0861a3d9ad841a61c626d7e7f8e2896bc 100644 (file)
--- a/js/ui/status/bluetooth.js
+++ b/js/ui/status/bluetooth.js
@@ -67,7 +67,6 @@ Indicator.prototype = {
                                new PopupMenu.PopupMenuItem(_("Set up a New Device...")),
                                new PopupMenu.PopupSeparatorMenuItem()];
         this._hasDevices = false;
-        this._deviceSep = this._fullMenuItems[0]; // hidden if no device exists
 
         this._fullMenuItems[1].connect('activate', function() {
             GLib.spawn_command_line_async('bluetooth-sendto');
@@ -162,10 +161,6 @@ Indicator.prototype = {
                 this._hasDevices = true;
             }
         }
-        if (this._hasDevices)
-            this._deviceSep.actor.show();
-        else
-            this._deviceSep.actor.hide();
     },
 
     _updateDeviceItem: function(item, device) {
@@ -303,8 +298,6 @@ Indicator.prototype = {
             this._showAll(this._fullMenuItems);
             if (this._hasDevices)
                 this._showAll(this._deviceItems);
-            else
-                this._deviceSep.actor.hide();
         } else {
             this._hideAll(this._fullMenuItems);
             this._hideAll(this._deviceItems);

diff --git a/js/ui/status/network.js b/js/ui/status/network.js
index 2f65d1a3a6063b56b96ddc9cc51186eca583fabf..447efffcf932e7b2bb944177d64654c6438b8075 100644 (file)
--- a/js/ui/status/network.js
+++ b/js/ui/status/network.js
@@ -1558,9 +1558,9 @@ NMApplet.prototype = {
         this._statusSection.addAction(_("Enable networking"), Lang.bind(this, function() {
             this._client.networking_enabled = true;
         }));
-        this._statusSection.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._statusSection.actor.hide();
         this.menu.addMenuItem(this._statusSection);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices = { };
 
@@ -1571,9 +1571,9 @@ NMApplet.prototype = {
         };
 
         this._devices.wired.section.addMenuItem(this._devices.wired.item);
-        this._devices.wired.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.wired.section.actor.hide();
         this.menu.addMenuItem(this._devices.wired.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices.wireless = {
             section: new PopupMenu.PopupMenuSection(),
@@ -1581,9 +1581,9 @@ NMApplet.prototype = {
             item: this._makeToggleItem('wireless', _("Wireless"))
         };
         this._devices.wireless.section.addMenuItem(this._devices.wireless.item);
-        this._devices.wireless.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.wireless.section.actor.hide();
         this.menu.addMenuItem(this._devices.wireless.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices.wwan = {
             section: new PopupMenu.PopupMenuSection(),
@@ -1591,9 +1591,9 @@ NMApplet.prototype = {
             item: this._makeToggleItem('wwan', _("Mobile broadband"))
         };
         this._devices.wwan.section.addMenuItem(this._devices.wwan.item);
-        this._devices.wwan.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.wwan.section.actor.hide();
         this.menu.addMenuItem(this._devices.wwan.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._devices.vpn = {
             section: new PopupMenu.PopupMenuSection(),
@@ -1606,9 +1606,9 @@ NMApplet.prototype = {
         this._devices.vpn.item.updateForDevice(this._devices.vpn.device);
         this._devices.vpn.section.addMenuItem(this._devices.vpn.item);
         this._devices.vpn.section.addMenuItem(this._devices.vpn.device.section);
-        this._devices.vpn.section.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._devices.vpn.section.actor.hide();
         this.menu.addMenuItem(this._devices.vpn.section);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this.menu.addAction(_("Network Settings"), function() {
             Main.overview.hide();

diff --git a/js/ui/status/power.js b/js/ui/status/power.js
index 482711bf5c2c44b3deb63d2cb4576ef5cacf124c..9b88ac371ff7ee7d0c5332667b1d15e7ca26368f 100644 (file)
--- a/js/ui/status/power.js
+++ b/js/ui/status/power.js
@@ -75,8 +75,7 @@ Indicator.prototype = {
         this._batteryItem.addActor(this._primaryPercentage, { align: St.Align.END });
         this.menu.addMenuItem(this._batteryItem);
 
-        this._deviceSep = new PopupMenu.PopupSeparatorMenuItem();
-        this.menu.addMenuItem(this._deviceSep);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
         this._otherDevicePosition = 2;
         this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
@@ -96,7 +95,6 @@ Indicator.prototype = {
                 this._hasPrimary = false;
                 this._primaryDeviceId = null;
                 this._batteryItem.actor.hide();
-                this._deviceSep.actor.hide();
                 return;
             }
             let [device_id, device_type, icon, percentage, state, seconds] = device;
@@ -126,12 +124,9 @@ Indicator.prototype = {
                 }
                 this._primaryPercentage.text = Math.round(percentage) + '%';
                 this._batteryItem.actor.show();
-                if (this._deviceItems.length > 0)
-                    this._deviceSep.actor.show();
             } else {
                 this._hasPrimary = false;
                 this._batteryItem.actor.hide();
-                this._deviceSep.actor.hide();
             }
 
             this._primaryDeviceId = device_id;
@@ -144,7 +139,6 @@ Indicator.prototype = {
             this._deviceItems = [];
 
             if (error) {
-                this._deviceSep.actor.hide();
                 return;
             }
 
@@ -159,11 +153,6 @@ Indicator.prototype = {
                 this.menu.addMenuItem(item, this._otherDevicePosition + position);
                 position++;
             }
-
-            if (this._hasPrimary && position > 0)
-                this._deviceSep.actor.show();
-            else
-                this._deviceSep.actor.hide();
         }));
     },
 

diff --git a/js/ui/status/volume.js b/js/ui/status/volume.js
index 98e8d30f6445240f621d463dd1a68078b80b1ec2..406ac66419b12f2c91dcc7802dddbe120021a7e8 100644 (file)
--- a/js/ui/status/volume.js
+++ b/js/ui/status/volume.js
@@ -47,8 +47,7 @@ Indicator.prototype = {
         this.menu.addMenuItem(this._outputTitle);
         this.menu.addMenuItem(this._outputSlider);
 
-        this._separator = new PopupMenu.PopupSeparatorMenuItem();
-        this.menu.addMenuItem(this._separator);
+        this.menu.addMenuItem(new PopupMenu.PopupSeparatorMenuItem());
 
         this._input = null;
         this._inputVolumeId = 0;
@@ -145,7 +144,6 @@ Indicator.prototype = {
             this._mutedChanged (null, null, '_input');
             this._volumeChanged (null, null, '_input');
         } else {
-            this._separator.actor.hide();
             this._inputTitle.actor.hide();
             this._inputSlider.actor.hide();
         }
@@ -168,11 +166,9 @@ Indicator.prototype = {
             }
         }
         if (showInput) {
-            this._separator.actor.show();
             this._inputTitle.actor.show();
             this._inputSlider.actor.show();
         } else {
-            this._separator.actor.hide();
             this._inputTitle.actor.hide();
             this._inputSlider.actor.hide();
         }

diff --git a/js/ui/statusMenu.js b/js/ui/statusMenu.js
index 21bc2b12877f3d298dbba8283b75376973adf800..2f16cbc423826ff991a15d20e2600bb271f36ae8 100644 (file)
--- a/js/ui/statusMenu.js
+++ b/js/ui/statusMenu.js
@@ -115,35 +115,12 @@ StatusMenuButton.prototype = {
           this._name.set_text("");
     },
 
-    _updateSessionSeparator: function() {
-        let sessionItemsVisible = this._loginScreenItem.actor.visible ||
-                                  this._logoutItem.actor.visible ||
-                                  this._lockScreenItem.actor.visible;
-
-        let showSessionSeparator = sessionItemsVisible &&
-                                   this._suspendOrPowerOffItem.actor.visible;
-
-        let showSettingsSeparator = sessionItemsVisible ||
-                                    this._suspendOrPowerOffItem.actor.visible;
-
-        if (showSessionSeparator)
-            this._sessionSeparator.actor.show();
-        else
-            this._sessionSeparator.actor.hide();
-
-        if (showSettingsSeparator)
-            this._settingsSeparator.actor.show();
-        else
-            this._settingsSeparator.actor.hide();
-    },
-
     _updateSwitchUser: function() {
         let allowSwitch = !this._lockdownSettings.get_boolean(DISABLE_USER_SWITCH_KEY);
         if (allowSwitch && this._gdm.can_switch ())
             this._loginScreenItem.actor.show();
         else
             this._loginScreenItem.actor.hide();
-        this._updateSessionSeparator();
     },
 
     _updateLogout: function() {
@@ -152,7 +129,6 @@ StatusMenuButton.prototype = {
             this._logoutItem.actor.show();
         else
             this._logoutItem.actor.hide();
-        this._updateSessionSeparator();
     },
 
     _updateLockScreen: function() {
@@ -161,7 +137,6 @@ StatusMenuButton.prototype = {
             this._lockScreenItem.actor.show();
         else
             this._lockScreenItem.actor.hide();
-        this._updateSessionSeparator();
     },
 
     _updateHaveShutdown: function() {
@@ -184,7 +159,6 @@ StatusMenuButton.prototype = {
             this._suspendOrPowerOffItem.actor.hide();
         else
             this._suspendOrPowerOffItem.actor.show();
-         this._updateSessionSeparator();
 
         // If we can't suspend show Power Off... instead
         // and disable the alt key
@@ -237,7 +211,6 @@ StatusMenuButton.prototype = {
 
         item = new PopupMenu.PopupSeparatorMenuItem();
         this.menu.addMenuItem(item);
-        this._settingsSeparator = item;
 
         item = new PopupMenu.PopupMenuItem(_("Lock Screen"));
         item.connect('activate', Lang.bind(this, this._onLockScreenActivate));
@@ -256,7 +229,6 @@ StatusMenuButton.prototype = {
 
         item = new PopupMenu.PopupSeparatorMenuItem();
         this.menu.addMenuItem(item);
-        this._sessionSeparator = item;
 
         item = new PopupMenu.PopupAlternatingMenuItem(_("Suspend"),
                                                       _("Power Off..."));